Skip to content

Conversation

@xiangyan99
Copy link
Member

No description provided.

@adxsdk6
Copy link

adxsdk6 commented Aug 12, 2019

Can one of the admins verify this patch?

@xiangyan99 xiangyan99 requested a review from mayurid as a code owner August 12, 2019 20:57
OSName: 'Linux'
OSVmImage: 'ubuntu-16.04'
PythonVersion: 'pypy3'
BuildTargetingString: $(BuildTargetingString)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! The variable $(BuildTargetString) is configurable at the pipeline level. You can safely put just your data-plane package name at the pipeline variable level. Can walk you through that if you need!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Because this tests.yml file is specific to appconfiguration it's ok to hardcode the value you want to use here instead of configuring it manually in the pipeline.

We want to avoid pipelines that require manual configuration because it makes pipeline automation more difficult.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean I can just hard code "azure-data-appconfiguration"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline. Set queue time variable on AzureDevOps: BuildTargetingString == "azure-data-appconfiguration"

Copy link
Member

@danieljurek danieljurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(actually submitting comments on this approval)

OSName: 'Linux'
OSVmImage: 'ubuntu-16.04'
PythonVersion: 'pypy3'
BuildTargetingString: $(BuildTargetingString)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Because this tests.yml file is specific to appconfiguration it's ok to hardcode the value you want to use here instead of configuring it manually in the pipeline.

We want to avoid pipelines that require manual configuration because it makes pipeline automation more difficult.

@xiangyan99 xiangyan99 merged commit 9f89084 into master Aug 13, 2019
@xiangyan99 xiangyan99 deleted the EnableLiveTests branch August 19, 2019 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants